Repository navigation
ci: add integration job and tests for acme-commerce-lenders - #20
localstack-platform[bot] wants to merge 9 commits into
Conversation
LocalStack PR Review — 🟢 GREEN
Checks · 1 passed · 0 failed · 0 errored
✓ No infrastructure changes✅ passed · Run #58 · CostFixed monthly, on-demand, USD · - · AWS prices of 2026-11-01 Fixed monthly cost: $0.00/month. No priced base yet: the next default-branch run sets one. View run in LocalStack · Open sandbox · updated for |
The generated localstack-ci.yml becomes an integration job in ci.yml, with the same pinned actions and fork guard. The job drives the demo over HTTP against LocalStack: seed facts, the full apply to settle flow, out of order steps, recorded vendor calls and reset. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
LocalStack PR Review — why this CI run failedLocalStack start failed: Omission of stripe from the app emulator fallback loop · confidence medium (no known signature matched; the agent's diagnosis has verified evidence) The CI workflow failed during emulator readiness verification because .github/workflows/ci.yml omitted stripe from the loop that checks and runs fallback app emulators (for app in plaid xero; do). Because the LocalStack instance did not include a hosted Stripe emulator, kit ready failed with an error indicating that stripe was not ready and STRIPE_EMULATOR_URL was not set. Adding stripe to the list ensures twin-stripe is built and started via WonderTwin when LocalStack does not host it. Evidence What to change
Diagnosed for |
Drops the WonderTwin fallback and the custom port. The job now fails at the ready check if LocalStack does not host an app emulator. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The test starts the demo in-process, so the job no longer builds the kit, runs it in the background or polls for it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
LocalStack PR Review — why this CI run failedLocalStack start failed: Tests failed · confidence high (matched a known signature; the agent agrees) The integration test suite failed during the test stage because the required vendor app emulators (Stripe, Plaid, and Xero) were not reachable or included in the sandbox environment. When TestSeededWorldIsReadyAndMatchesTheDocs called /api/reset, requests routed to *.localhost.localstack.cloud:4566 returned S3 NoSuchBucket 404 errors, causing the test suite to receive 502 response codes and fail. Evidence What to change
Diagnosed for |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Heads up: the "Run the app emulators LocalStack does not host" step in the integration job is a temporary workaround, and we should remove it as soon as we can. Why it's there: the hosted LocalStack sandbox doesn't serve all the emulators the demo needs.
So for now the step builds whichever of stripe, plaid and xero LocalStack doesn't answer for from WonderTwin and points the demo at them via Remove it once:
The step checks each vendor on LocalStack first and only builds the ones that don't answer, so it will quietly stop doing anything once LocalStack covers them. It is still extra build time and a dependency on the WonderTwin repo, so it should go rather than stay forever. The "Probe the app emulators" step above it is a debugging aid and can go at the same time. |
|
Thanks for this, @whummer . The tests are the valuable part: a full Four things I'd like in before merge (details inline):
Items 2 and 3 are one change. Everything else can be a follow-up. |
tela
left a comment
tela
left a comment
There was a problem hiding this comment.
On the description: it says this adds .github/workflows/localstack-ci.yml, lists "Emulator: AWS" and "Tests: none", and asks for a LOCALSTACK_AUTH_TOKEN repo secret. The diff edits ci.yml, adds integration_test.go, and authenticates through OIDC, so no secret is needed. Someone following the checklist would add one for nothing.
Could you rewrite it in the Motivation / Changes / Tests / Related format? Tests should show the exact go test -tags integration command and the green run. Related should carry the removal conditions from your comment. A title like "ci: add integration job and tests for acme-commerce-lenders" would fit.
Side note: the bot's "GREEN" review only checks an STS call, which says nothing about this demo. Worth ignoring as a signal.
| git clone --depth 1 https://github.com/WonderTwin-AI/wondertwin "$RUNNER_TEMP/wondertwin" | ||
| port=5101 | ||
| for app in $missing; do | ||
| (cd "$RUNNER_TEMP/wondertwin" && go build -o "$RUNNER_TEMP/twin-$app" "./twin-$app/cmd/twin-$app") | ||
| "$RUNNER_TEMP/twin-$app" --port=$port > "$RUNNER_TEMP/twin-$app.log" 2>&1 & |
There was a problem hiding this comment.
Two changes here.
Token: the OIDC step puts LOCALSTACK_AUTH_TOKEN in GITHUB_ENV, so every twin started below inherits it, and they are built from whatever WonderTwin main is at run time. They never need the token. Please start them as:
env -u LOCALSTACK_AUTH_TOKEN "$RUNNER_TEMP/twin-$app" --port=$port ...
Pin: git clone --depth 1 takes the default branch, so any WonderTwin change can turn this repo's CI red, and the test asserts exact numbers (941 orders, 8108384 gross, ...). Please pin to a commit. For example, set WONDERTWIN_REF at the top of the job and fetch that SHA. WonderTwin main at the time of the green run was 8f1b30f (inferred from the run time, the log does not record it). Bump it deliberately, in the PR that changes any asserted numbers.
|
|
||
| func TestMain(m *testing.M) { os.Exit(run(m)) } | ||
|
|
||
| func run(m *testing.M) int { |
There was a problem hiding this comment.
Every test here calls resetWorld, which wipes the app emulators. The package comment says "run lstk, then go test -tags integration". Run locally, that wipes everything in LocalStack's shared Stripe emulator, and in any Plaid or Xero emulator at _EMULATOR_URL.
Please add a guard at the top of run():
if os.Getenv("INTEGRATION_ALLOW_RESET") != "1" {
fmt.Fprintln(os.Stderr, "this resets every app emulator it talks to; set INTEGRATION_ALLOW_RESET=1 to run")
return 1
}
Then set it on the go test step in ci.yml and mention it in the package comment.
| # localstack-project-id: proj_5f62a1e8 (https://v2.app.localstack.cloud/projects/proj_5f62a1e8) | ||
| # Runtime config (emulator image, env vars, extensions) is fetched by | ||
| # `lstk project start` at run time, so change it on the platform, not here. | ||
| integration: |
There was a problem hiding this comment.
Three smaller things, none blocking:
- In the run log stripe, plaid and xero all answer 404 on LocalStack, so today all three come from WonderTwin builds and LocalStack is started but not exercised. Could the job comment say that, so nobody reads this as LocalStack-hosted coverage?
- Please open a tracking issue for removing the workaround step and the probe step, with your two conditions as the checklist (plaid and xero released in LocalStack, sandbox image new enough to serve stripe).
- 8 commits, several of them CI debugging. Fine to squash-merge into one. Separately: the
lstkinstall is unpinned and the wheel comes from the localstack-artifacts-internal bucket with no hash, in a public repo. Worth confirming with the platform team that this is intended.
- Pin WonderTwin to a commit instead of cloning main, so upstream changes cannot turn this repo's CI red. - Start the twins without LOCALSTACK_AUTH_TOKEN. - Require INTEGRATION_ALLOW_RESET=1 to run the integration tests, since they reset every app emulator they talk to. - Note in the job comment that all three emulators come from WonderTwin today. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
whummer
changed the title
|
Thanks for the thorough review, @tela. Pushed
CI is green on |
Motivation
The
acme-commerce-lendersdemo had no test that drives the whole flow against real app emulators. This adds an integration job to CI and Go integration tests, so the docs and the emulators can't drift apart.Changes
.github/workflows/ci.yml: newintegrationjob. It authenticates to the LocalStack platform through GitHub OIDC (no repository secret needed), starts the project withlstk project start, and runs the tests. Fork PRs skip the job because they don't get an OIDC token.demos/acme-commerce-lenders/integration/integration_test.go: a full seed, apply, accept, fund and settle run over HTTP. It also checks the 409 on out-of-order steps and asserts the documented numbers.INTEGRATION_ALLOW_RESET=1is set. CI sets it.WONDERTWIN_REF, and starts them without the LocalStack token. LocalStack is started but not exercised, so this is not LocalStack-hosted coverage yet.Tests
All three CI jobs (
build-and-test,web,integration) are green oncabf9c4: https://github.com/localstack-samples/localstack-app-emulators/actions/runs/42910568236Related
Removal conditions for the workaround and probe steps are tracked in #24: plaid and xero released in LocalStack, and a sandbox image new enough to serve stripe.
🤖 Generated with Claude Code